Skip to content

fix: apply signer removal to every document in an envelope - #9101

Merged
vitormattos merged 2 commits into
LibreSign:mainfrom
carlomarmo:fix/envelope-signer-removal-9085
Oct 20, 2026
Merged

vitormattos merged 2 commits into
LibreSign:mainfrom
carlomarmo:fix/envelope-signer-removal-9085

Conversation

@carlomarmo

Copy link
Copy Markdown
Contributor

Removing someone from an envelope currently leaves their signing requests open on the documents inside it. Remove the last signer and the envelope returns to draft, but its documents still wait for a signature. The person managing the package cannot rely on the envelope's signer list matching its documents.

The removal code was deleting the signer's identification details before using them to find the matching requests on the documents. This fix reuses the details already read before deletion, so removal reaches every document and leaves the other signers alone.

Two added tests cover removing one of two signers and removing the last signer. Both fail without the fix; all 15 focused Behat scenarios pass with it. PHP syntax and whitespace checks also pass.

Checked locally with PHP 8.3.6, Nextcloud 36 (development version) and SQLite. PHPUnit, static analysis, GitHub CI and the full supported-version matrix have not run.

Related to #9085.

@carlomarmo
carlomarmo requested a review from a team as a code owner October 16, 2026 12:34
@welcome

welcome Bot commented Oct 16, 2026

Copy link
Copy Markdown

Thanks for opening your first pull request in this repository! ✌️

@YvesCesar
YvesCesar self-requested a review October 16, 2026 14:43

@YvesCesar YvesCesar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @carlomarmo !

I made a few revisions. Could you take a look?

Comment thread lib/Service/RequestSignatureService.php
Comment thread lib/Service/RequestSignatureService.php
@carlomarmo

Copy link
Copy Markdown
Contributor Author

Thanks @YvesCesar. Removing a signer from the envelope now sends one cancellation notification, email and activity entry. Removing a signer directly from a document still notifies as before. I've added tests for both and moved the empty check before the child lookup.

@YvesCesar
YvesCesar self-requested a review October 17, 2026 15:06
@YvesCesar

Copy link
Copy Markdown
Member

@carlomarmo thanks for your contribution.

@vitormattos , that looks okay to me.

I don't think these errors in the CI have anything to do with the PR.

Keep the signer's identification details available when removing their
requests from the documents in an envelope.

Add tests for removing one of two signers and removing the last signer.

Refs LibreSign#9085

Signed-off-by: Carlo Marmo <carlo.marmo@gmail.com>
Send the cancellation event for the envelope, not each document updated
with it. Keep notifications for direct document removals.

Check the email, notification and activity counts, and skip the child
lookup when there are no identification methods.

Signed-off-by: Carlo Marmo <carlo.marmo@gmail.com>
@vitormattos
vitormattos force-pushed the fix/envelope-signer-removal-9085 branch from 5bd8a4c to e165e0c Compare October 20, 2026 00:41
@vitormattos vitormattos added this to the Next Major (36) milestone Oct 20, 2026 — with ChatGPT Codex Connector
@vitormattos
vitormattos merged commit fb0b96f into LibreSign:main Oct 20, 2026
67 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Hi @carlomarmo, your first pull request to LibreSign has been merged. 🎉

Your work is part of LibreSign now, and everyone who signs a document with it benefits from the time and knowledge you shared. Thank you.

You are very welcome to contribute again. The good first issue list is a good place to look next, and you no longer have to wonder how the process works.

If you have a few minutes and feel like it, we would love to hear how this went for you: https://ls.librecode.coop/index.php/645932?lang=en&source=github-first-merged-pr&repository=libresign. It is entirely optional; nothing is expected of you in return for contributing.

Come say hello in the LibreSign community: https://t.me/LibreSign

vitormattos added a commit that referenced this pull request Oct 20, 2026
Backport the Behat cancellation scenarios from #9101, including removal
from child documents, last signer cleanup, and notification counts.

Original contribution by Carlo Marmo in #9101.

Co-authored-by: Carlo Marmo <carlo.marmo@gmail.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos added a commit that referenced this pull request Oct 20, 2026
Backport the Behat cancellation scenarios from #9101, including removal
from child documents, last signer cleanup, and notification counts.

Original contribution by Carlo Marmo in #9101.

Co-authored-by: Carlo Marmo <carlo.marmo@gmail.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos added a commit that referenced this pull request Oct 20, 2026
Backport the Behat cancellation scenarios from #9101, including removal
from child documents, last signer cleanup, and notification counts.

Original contribution by Carlo Marmo in #9101.

Co-authored-by: Carlo Marmo <carlo.marmo@gmail.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants